Stop panicking in Current()
#81
Conversation
Instead, just return an error, so it's up to the calling code how to handle it. This is an API-breaking change. Signed-off-by: Marat Radchenko <[email protected]>
slonopotamus
force-pushed
the
do-not-panic
branch
from
309bc31 to
c641c39
Compare
| // Current returns the current process' console | ||
| func Current() (c Console) { | ||
| var err error | ||
| func Current() (c Console, err error) { |
There was a problem hiding this comment.
Instead of renaming this, can we add an additional constructor that returns the error? Something like:
func GetCurrent() (Console, error)There was a problem hiding this comment.
Then we shouldn't have to worry about the breaking change.
There was a problem hiding this comment.
👍 this code may be incorporated in various projects, and we should try to avoid breaking this signature (it may be complex to resolve otherwise). I don't have a good suggestion for the new function's name (perhaps someone else has?). Perhaps TryCurrentConsole() ?
That said, it looks like Current() is a very small wrapper around ConsoleFromFile(), so to some extent, users would already be able to work around this.
We should improve the GoDoc for Current() though!
- Outline what it does (tries the standard-streams (STDERR, STDOUT, STDIN to get the console)
- It panics when it fails to detect a console in any of those.
- Point users to the alternative / equivalent using
ConsoleFromFile
|
I'm not interested in pushing these changes forward anymore. My goal was to use |
3 participants
Instead, just return an error, so it's up to the calling code how to handle it.
This is an API-breaking change.